Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix failing master lint #3640

Merged
merged 11 commits into from
Jun 13, 2022
Merged

Fix failing master lint #3640

merged 11 commits into from
Jun 13, 2022

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Jun 12, 2022

  • Fix husky
  • Fix css lint
  • Fix ts lint

Not sure when everything started breaking. Husky probably broke when #3396 was merged since that was when husky was upgraded. Maybe the CSS lint and TS lint broke at that time as well since we did upgrade a lot of different packages 🤔 Have to run some tests to be sure!

@julieg18 julieg18 temporarily deployed to dvc-org-fix-failing-mas-kgq5zh June 12, 2022 20:46 Inactive
@julieg18 julieg18 marked this pull request as draft June 12, 2022 20:47
@julieg18 julieg18 self-assigned this Jun 12, 2022
@julieg18 julieg18 temporarily deployed to dvc-org-fix-failing-mas-kgq5zh June 12, 2022 21:00 Inactive
@julieg18 julieg18 temporarily deployed to dvc-org-fix-failing-mas-kgq5zh June 12, 2022 22:29 Inactive
@julieg18 julieg18 temporarily deployed to dvc-org-fix-failing-mas-kgq5zh June 12, 2022 23:29 Inactive
@@ -21,7 +21,8 @@
"link-check": "repo-link-check -c config/link-check/config.yml",
"link-check-diff": "repo-link-check -c config/link-check/config.yml -d master",
"link-check-dev-server": "repo-link-check -c config/link-check/config.yml -r http://localhost:3000",
"link-check-exclude": "repo-link-check -c config/link-check/config.yml --unused-patterns-only"
"link-check-exclude": "repo-link-check -c config/link-check/config.yml --unused-patterns-only",
"prepare": "husky install"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Husky works different after v8! See husky docs

@@ -111,7 +112,6 @@
"@types/react-instantsearch-dom": "^6.12.3",
"@types/react-popover": "^0.5.3",
"@types/react-slick": "^0.23.8",
"@types/rehype-react": "^6.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that we no longer need this package and deleted it.

@@ -1,4 +1,4 @@
import React from 'react'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some errors were simple to fix.

@@ -1,17 +1,17 @@
.cmlIcon {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how stylelint missed this but it's fixed now!

@@ -20,6 +24,7 @@ const Popover: React.FC<IPopoverProps> = ({ children, ...restProps }) => {
const close = (): void => setOpened(false)

return (
// @ts-expect-error Error is caused by package and out of our control
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out how to clear this error because it seemed to be caused by the fact that ReactPopover doesn't expect children. But the package here isn't very well maintained... The only thing I could come up with was ignoring the error but does anyone have a better alternative?

No overload matches this call.
  Overload 1 of 2, '(props: PopoverProps | Readonly<PopoverProps>): Popover', gave the following error.
    Type '{ children: Element; body: ReactNode; enterExitTransitionDurationMs: number; isOpen: boolean; onOuterAction: () => void; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Popover> & Readonly<PopoverProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Popover> & Readonly<PopoverProps>'.
  Overload 2 of 2, '(props: PopoverProps, context: any): Popover', gave the following error.
    Type '{ children: Element; body: ReactNode; enterExitTransitionDurationMs: number; isOpen: boolean; onOuterAction: () => void; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Popover> & Readonly<PopoverProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Popover> & Readonly<PopoverProps>'.

@@ -21,5 +21,6 @@
"alwaysStrict": true,
"noEmit": true
},
"include": ["./src/**/*", "./*.js"]
"include": ["./src/**/*", "./*.js"],
"exclude": ["./node_modules/**/*"]
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good chunk of errors were based around PACKAGE_NAME cannot be used as a JSX component or NODE_MODULE PATH.ReactNode' is not assignable to type 'React.ReactNode'.

I found this StackOverflow thread but adding resolutions didn't work. I ended up deleting yarn.lock and reinstalling which cleared all the errors. I looked over the site and everything looks built correctly 🤔 But if anyone has a better idea to clearing the errors, please let me know!

node_modules/@dvcorg/gatsby-theme-iterative/src/components/MainLayout/index.tsx:59:9 - error TS2322: Type 'import("/home/circleci/repo/node_modules/@dvcorg/gatsby-theme-iterative/node_modules/@types/react/index").ReactNode' is not assignable to type 'React.ReactNode'.

node_modules/@dvcorg/gatsby-theme-iterative/src/components/Page/DefaultSEO/index.tsx:105:6 - error TS2786: 'Helmet' cannot be used as a JSX component.
  Its instance type 'Helmet' is not a valid JSX element.

@julieg18 julieg18 requested a review from a team June 12, 2022 23:53
@julieg18 julieg18 marked this pull request as ready for review June 12, 2022 23:53
@julieg18 julieg18 merged commit 09531a8 into master Jun 13, 2022
@julieg18 julieg18 deleted the fix-failing-master-lint branch June 13, 2022 18:27
@jorgeorpinel
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants